Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes early destruction of IAsyncEnumerable<T> sent as return value from RPC methods #1004

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

AArnott
Copy link
Member

@AArnott AArnott commented Feb 13, 2024

In certain scenarios, there may have also been effectively a memory leak that this also fixes.

Fixes #999

This pull request primarily involves changes to the src/StreamJsonRpc/Reflection/MessageFormatterEnumerableTracker.cs file and the test/StreamJsonRpc.Tests/AsyncEnumerableTests.cs file. The changes aim to improve the tracking of outbound request IDs and their corresponding IAsyncEnumerable<T> state machines, and to update the test cases to reflect these changes.

Key changes include:

MessageFormatterEnumerableTracker.cs:

  • The dictionary used to map outbound request IDs has been updated to track the list of tokens that represent IAsyncEnumerable<T> state machines it owns. This ensures that the state machines are cleaned up after receiving the final response.

  • The GetToken<T>(IAsyncEnumerable<T> enumerable) method has been updated to only track the token if it is serializing a request. This prevents outbound responses that carry enumerables from being disposed of when an inbound response with the same ID is received.

  • The CleanUpResources(RequestId requestId) method has been updated to clean up resources associated with outbound request IDs.

AsyncEnumerableTests.cs:

  • The AsyncEnumerableTests class has been updated to include a new Client object and a serverProxy for the IClient interface. [1] [2] [3] [4] [5] [6]

  • A new test case EnumerableIdDisposal() has been added to test the disposal of enumerable IDs.

  • The Server class has been updated to include a Client property and a new method CallbackClientAndYieldOneValueAsync(CancellationToken cancellationToken). [1] [2]

  • A new Client class has been added to implement the IClient interface.

… from RPC methods

In certain scenarios, there may have also been effectively a memory leak that this also fixes.

Fixes microsoft#999
@AArnott AArnott added this to the v2.18 milestone Feb 13, 2024
@AArnott AArnott requested a review from BertanAygun February 13, 2024 20:27
@AArnott AArnott self-assigned this Feb 13, 2024
@AArnott AArnott merged commit 85ac63f into microsoft:main Feb 13, 2024
6 checks passed
@AArnott AArnott deleted the fix999 branch February 13, 2024 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IAsyncEnumerable handler can be cleaned up early
2 participants